Why did you chose to put post_message in graph.ex while usually the whole API functions are in facebook.ex with clear parameters to create the payload? I don't currently see a reason to add a specialised post method which just has a predefined URL and content type but the arguments are so generic..?

You need a specialised post function because the messenger API requires the application/json header to be set, and a new function is the only way to pass in those headers without changing the arity of an existing in-use function

If you're okay with changing the existing post function to allow for passing in headers then I'll do that. I'm just aiming for zero impact area to begin with.

I see. I wonder if we couldn't just set that Content Type by default on the post function? The post video is already separate so that should be ok I assume?

We can do it if you're ok with changing all calls to post to add a [] to the argument list, but if you mean default as in without changing the calling code, then it's finicky.

If we add a parameter with a default value headers \\ [content_type: "application/JSON"] then the default post and the post video function end up with the same arity and the compiler complains.

It wants:

def post(path, payload, params, options, headers \\ default)
def post(path, payload, params, options, headers) do ... end
def post(:video, path, payload, params, options) do .. end

But the problem with that is you get the default for headers but the top bodyless clause arguments are out of sync with the arguments of post(:video, ...) which is pretty clunky

And if we set the headers inside the function body then we're locked in to always using JSON. The messenger API doesn't take anything else, but other endpoints might.

Sorry for replying late.

We can do it if you're ok with changing all calls to post to add a [] to the argument list, but if you mean default as in without changing the calling code, then it's finicky.

Let's do that. 👍

In #44 I brought up the idea of replacing the Graph module with HTTPoison. I've only experimented with this mildly, and was too fearful of breaking stuff, but, if implemented, it would let us pass headers directly to the http library in a consistent way that's already documented for us. We could also configure the default headers in the module that wraps HTTPoison.Base and override them where necessary.

I was procrastinating on making these changes until the open PR's were merged, but given this could potentially save some effort here, I will make something public sooner.

@Potrimpo could you rebase your changes on master please?

ping?

@Potrimpo could you please update your PR to the latest changes? Otherwise I'll have to close this PR.